Skip to content

fix: improve proxy for images without content-type#2740

Open
alexdln wants to merge 2 commits into
npmx-dev:mainfrom
alexdln:fix/proxy-without-content-type
Open

fix: improve proxy for images without content-type#2740
alexdln wants to merge 2 commits into
npmx-dev:mainfrom
alexdln:fix/proxy-without-content-type

Conversation

@alexdln
Copy link
Copy Markdown
Member

@alexdln alexdln commented May 13, 2026

🧭 Context

Some services don't specify the content-type (or uses application/octet-stream), leaving it up to the user.
In these cases, we immediately return an error, although even standard providers such as github do this (example package - https://npmx.dev/package/ag-grid-react)

📚 Description

Updated the logic to additionally check the extension. This doesn't compromise security, as both the extension and the content-type allow the user to forge the value.

In the example package above, one gif still doesn't work because it's above our limit (and we should leave it that way), but the other 3 works

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment May 29, 2026 4:24pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview May 29, 2026 4:24pm
npmx-lunaria Ignored Ignored May 29, 2026 4:24pm

Request Review

@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 25f3c92e-33af-4daa-a60a-8a6cbc957f07

📥 Commits

Reviewing files that changed from the base of the PR and between cf9bd5b and 7f457fd.

📒 Files selected for processing (1)
  • server/api/registry/image-proxy/index.get.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • server/api/registry/image-proxy/index.get.ts

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved image proxy handling: it now accepts a wider range of image responses, including generic binary responses when the requested URL clearly points to an image (common extensions like .png, .jpg, .jpeg, .gif). This increases compatibility with varied image sources and reduces false rejections.

Walkthrough

The PR enhances the image-proxy endpoint's content-type validation by introducing permissive detection. It now accepts application/octet-stream responses as valid images when the requested URL pathname ends with recognised image extensions, alongside the existing image/* content-type acceptance.

Changes

Image Proxy Content-Type Validation

Layer / File(s) Summary
Pathname-based image content-type acceptance
server/api/registry/image-proxy/index.get.ts
Extracts the requested URL pathname and introduces an isImageLike flag to treat application/octet-stream responses as acceptable when the pathname ends with common image extensions (.png, .jpg, .jpeg, .gif). The response validation guard is updated to combine this pathname-based check with the existing image/* content-type check.

Suggested reviewers

  • ghostdevv
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: improving the image proxy to handle images without proper content-type headers by checking file extensions.
Description check ✅ Passed The description is well-related to the changeset, providing context about why the change was needed and explaining the fallback extension-checking logic implemented.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@server/api/registry/image-proxy/index.get.ts`:
- Around line 159-164: The extension check in
server/api/registry/image-proxy/index.get.ts is case-sensitive and blocks valid
image URLs like .PNG/.JpG; update the validation in the image-proxy GET handler
(the block that uses endsWith against the extensions array) to perform a
case-insensitive check by normalizing the URL/pathname or filename to lowercase
before calling endsWith (or replace endsWith checks with a case-insensitive
regex), ensuring the same allowed extensions list is used but comparisons ignore
case.
- Around line 157-166: The MIME comparison in the image proxy handler
(index.get) uses an exact match against "application/octet-stream", which misses
values with parameters like "application/octet-stream; charset=binary"; update
the logic that reads the response Content-Type (e.g., the variable holding the
MIME/type) to normalize it by lowercasing and splitting on ";" then trimming the
media type before doing the equality check and fallback decision; replace direct
comparisons to "application/octet-stream" with comparisons against this
normalizedMediaType so the fallback path triggers correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 45ffe7cd-11ac-4663-a8c2-3afb6bae8b7a

📥 Commits

Reviewing files that changed from the base of the PR and between c2c2754 and cf9bd5b.

📒 Files selected for processing (1)
  • server/api/registry/image-proxy/index.get.ts

Comment thread server/api/registry/image-proxy/index.get.ts Outdated
Comment thread server/api/registry/image-proxy/index.get.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants